-
Notifications
You must be signed in to change notification settings - Fork 213
Bug 1841239: Avoid pre-creating clusteroperators that should be excluded #376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 1841239: Avoid pre-creating clusteroperators that should be excluded #376
Conversation
|
@csrwng: This pull request references Bugzilla bug 1841239, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/lgtm Looks like a clean backport to me. Was there a reason you didn't comment with |
|
/hold Unifying with #348 per this comment makes sense to me. |
Premature optimization :) I thought the patch wouldn't apply cleanly.
agree |
|
Unifying with #348 per this comment makes sense to me as well. |
aadb13e to
e4e771c
Compare
|
/test e2e-aws |
|
Hrm, #348 never had a backing bug. Which probably means we need to grow the QE verification scope on this series' 4.5 bug so they confirm that functionality is working as expected too. Or are we just going to assume "create ClusterOperators early" works and not cover it in QE for 4.5 or 4.4? Also, I'm not sure why the update job failed. Is there a list of failing vs. flaky/optional failures in the build log that I'm just missing? |
I think the right thing to do is to make the bug about introducing the "create clusteroperators early" functionality in 4.4. The other part is just a corollary fix so that it works properly for ROKS as well. Not sure how that works as far as depencies go though. There was no BZ to introduce the function in 4.5, but I think the 4.4 bug requires a 4.5 bug to exist. |
Looks like this is missing a backport of openshift/machine-config-operator#1566 |
|
Just posted a quick note to bug 1841239, QE still has to verify the 4.5 bug anyway and I'm sure once they do that they'll understand everything that's going on in this PR and the linked bug. |
Hmm, starting to think we should call in @deads2k to round up all the follow up work that should come along with backporting the CVO pre-fill? This is just docs but it's linked from the original PR too #363 |
|
looks faithful. I don't feel strongly about a readme /lgtm |
|
/hold |
|
@sdodson: This pull request references Bugzilla bug 1841239, which is valid. 6 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
We've provided QE notes in the bug that I believe explain the situation sufficiently. If they have questions I expect them to ask. |
|
/hold cancel |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/hold |
The must-gather and insights operator depend on cluster operators and related objects in order to identify resources to create. Because cluster operators are delegated to the operator install and upgrade failures of new operators can fail to gather the requisite info if the cluster degrades before those steps. Add a new selective Precreating install mode and do a single pass over all cluster operators in the payload without retries at the beginning of an initializing or upgrading sync pass to attempt to create the ClusterOperators if they don't exist. If we succeed at creating the object, try exactly once to update status so that relatedObjects can be set.
e4e771c to
44b7cb9
Compare
|
rebased, fixed |
|
/test images |
|
/hold cancel |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: csrwng, deads2k, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/bugzilla refresh |
|
@eparis: This pull request references Bugzilla bug 1841239, which is valid. 6 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@csrwng: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@csrwng: All pull requests linked via external trackers have merged: Bugzilla bug 1841239 has been moved to the MODIFIED state. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Backport of #370